Skip to content

chore(python,example): exampleにshebangを付けてexecutableにする#1077

Merged
qryxip merged 4 commits into
VOICEVOX:mainfrom
qryxip:pr/chore-python-example-make-examples-executable
May 1, 2025
Merged

chore(python,example): exampleにshebangを付けてexecutableにする#1077
qryxip merged 4 commits into
VOICEVOX:mainfrom
qryxip:pr/chore-python-example-make-examples-executable

Conversation

@qryxip
Copy link
Copy Markdown
Member

@qryxip qryxip commented Apr 26, 2025 •
edited
Loading

内容

run{,-asyncio}.pyにshebangを付け、filemodeも100755 (GIT_FILEMODE_BLOB_EXECUTABLE)にする。

shebangをpython3ではなくpythonにしているのは、仮想環境を前提にしたいから。

関連 Issue

その他

@qryxip qryxip requested a review from Hiroshiba April 26, 2025 05:34
@qryxip
Copy link
Copy Markdown
Member Author

qryxip commented Apr 26, 2025

https://github.com/VOICEVOX/voicevox_core/actions/runs/14678209867/job/41197509281?pr=1077

error: could not compile `proc-macro2` (lib) due to 3 previous errors

#1078

Hiroshiba
Hiroshiba reviewed Apr 26, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@Hiroshiba Hiroshiba left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

あーーーどうだろう。
これ逆に初学者が見たら混乱する気がしなくもないですねぇ…
付けないといけないのか、みたいに感じそう。

他のリポジトリでもつけてないし、特に理由なければつけない方が良い気がしました。
別にコアでだけつけるとかでも良いと思いますが、つけない方が一般的な気もしました！

@qryxip
Copy link
Copy Markdown
Member Author

qryxip commented May 1, 2025 •
edited
Loading

個人的には一般的な気がしています。Bashと同じような感覚。ONNX Runtimeなんかでも結構あったはず。

逆に付けないことによるデメリットとして、MSYS環境（e.g. Git Bash）にてシェルスクリプトとして実行されうるというのもあるはずかなと。

❯ echo 'print("hi")' > ./a.py
❯ # MSYSだと、Git側のfilemodeに関わらずそのまま実行可能になってしまう
❯ ./a.py
./a.py: line 1: syntax error near unexpected token `"hi"'
./a.py: line 1: `print("hi")'

@qryxip
Copy link
Copy Markdown
Member Author

qryxip commented May 1, 2025 •
edited
Loading

9c859b1 (#1077)
a2e2f3d (#1077)
ドキュメントとGHAにおけるpython ./run.pyを./run.pyにしました。

@qryxip qryxip force-pushed the pr/chore-python-example-make-examples-executable branch from 9c859b1 to 1e4316c Compare May 1, 2025 11:34
@qryxip qryxip force-pushed the pr/chore-python-example-make-examples-executable branch from 1e4316c to a2e2f3d Compare May 1, 2025 11:35
@Hiroshiba
Copy link
Copy Markdown
Member

Hiroshiba commented May 1, 2025 •
edited
Loading

うーーーん！
実行にpythonつけないのはだいぶ反対です！
windowsのコマンドライン等でも動くんですかね…？

あとシバンつけることに対してちょっとセカンドオピニオンが欲しい。
ということてご意見伺えると！！ @sevenc-nanashi

@qryxip
Copy link
Copy Markdown
Member Author

qryxip commented May 1, 2025

あ、そっかpoetry runとかも駄目だけどPowerShellとかも普通に駄目ですね。
c8c6984 (#1077): というわけでリバート

@qryxip
Copy link
Copy Markdown
Member Author

qryxip commented May 1, 2025

あ、いやcmdやPowerShellだったらexplorer.exe .\a.py相当にはなるのか。ただshebangを全くrespectしないことには変わりない。

sevenc-nanashi
sevenc-nanashi approved these changes May 1, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@sevenc-nanashi sevenc-nanashi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shebang追加だけなら大丈夫だと思います。

@qryxip qryxip requested a review from Hiroshiba May 1, 2025 13:04
Hiroshiba
Hiroshiba approved these changes May 1, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@Hiroshiba Hiroshiba left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM！！

（メモ）
個人的には1行目にshebangを書くのは相当珍しいと思っているので、VOICEVOX Org全体としてこちらに揃えたいという気持ちはないです。
けどコアはこういう運用というのは別にそれでも良さそう！

@qryxip qryxip merged commit ff06440 into VOICEVOX:main May 1, 2025
29 checks passed
@qryxip qryxip deleted the pr/chore-python-example-make-examples-executable branch May 1, 2025 17:36
@qryxip qryxip mentioned this pull request May 4, 2025
Merged
qryxip added a commit that referenced this pull request May 4, 2025
@qryxip
ci: check-shebangs-and-filemodesワークフローを追加 (#1080)
f4724c0 
#1077 の続き。

このリポジトリ内のすべてのファイルに対し、以下のチェックを義務付ける。

```bash
if has_shebang && ! is_executable; then
  echo 'A shebanged file must be executable (note: if you are using Windows, please change the filemode with `git update-index --chmod+x`)'
  exit 1
fi

if is_shellscript && ! is_executable; then
  echo 'A shell script file must be executable (note: if you are using Windows, please change the filemode with `git update-index --chmod+x`)'
  exit 1
fi

if is_executable && ! has_shebang; then
  echo 'An executable blob must have a shebang'
  exit 1
fi
```
@qryxip
Copy link
Copy Markdown
Member Author

qryxip commented Nov 5, 2025 •
edited
Loading

VOICEVOX/voicevox_engine#1818 の話で思ったけどこれ、#!/usr/bin/env -S poetry run pythonでもよかったかも。

@qryxip qryxip mentioned this pull request Nov 5, 2025
Closed
3 tasks
@qryxip
Copy link
Copy Markdown
Member Author

qryxip commented Nov 5, 2025 •
edited
Loading

いや、Python exampleはmaturin develop下だけで動く想定じゃなかった。GitHub Releases(あと将来的にPyPI)からvoicevox_coreパッケージをインストールしたとき用にも動かなければいけない。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Hiroshiba Hiroshiba Hiroshiba approved these changes

@sevenc-nanashi sevenc-nanashi sevenc-nanashi approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@qryxip @Hiroshiba @sevenc-nanashi